Skip to content

[skip changelog] Split publish docs workflow #1028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 15, 2020

Conversation

silvanocerza
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce?
    Splits the publish-docs.yaml workflow in two different files to handle different branches correctly.
  • What is the current behavior?
    Pushing to master triggers the workflow only if commited files match the following paths:
      - "docs/**"
      - "docsgen/**"
      - "cli/**"
      - "rpc/**"
      - ".github/workflows/publish-docs.yaml"

The same happens when pushing to a branch that matches the [0-9]+.[0-9]+.x regex like 0.12.x, 0.13.x, etc.

  • What is the new behavior?
    The behavior when pushing to master is unchanged.
    Pushing to a branch that matches the above regex always trigger the workflow.
  • Does this PR introduce a breaking change?
    No.
  • Other information:
    None.

See how to contribute

@silvanocerza silvanocerza added the topic: documentation Related to documentation for the project label Oct 14, 2020
@silvanocerza silvanocerza requested review from per1234 and rsora October 14, 2020 11:02
@silvanocerza silvanocerza self-assigned this Oct 14, 2020
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, the problem is that the docs are not published when the release branch is created, but only when a push is made to the release branch that modifies a file matching the on.push.paths filter. So what we really need is to trigger the workflow on the release branch creation. After that, it's fine to continue with the current behavior of only publishing when pushes are made to the release branch that modify a file matching the on.push.paths filter

I have an alternative proposal:
https://github.com/per1234/arduino-cli/commit/6d9bfd3ba70155ba7e2eace4edc58dd2a5511ad8

It uses the create event to trigger the workflow on every branch or tag creation. Unfortunately, the branches filter is not supported for this event. Also, filter patterns aren't supported in the jobs.<job_id>.if key. So the job will run on every branch or tag creation. The solution is to use a step with some bash code to do the filter:

RELEASE_BRANCH_REGEX="refs/heads/[0-9]+.[0-9]+.x"
if [[ "${{ github.event_name }}" == "push" || ( "${{ github.event_name }}" == "create" && "${{ github.ref }}" =~ $RELEASE_BRANCH_REGEX) ]]; then

then use the output of that job in the conditional of the publish job.

It's a little messy, but I think it's slightly better than maintaining duplicate workflows.

Let me know what you think.

@silvanocerza
Copy link
Contributor Author

Uuuh yes, your solution is certainly better! Didn't think about using the create event for this.
Feel free to push the changes directly to this branch, or if you want I can cherry pick the change from your repo. Let me know.

Previously, release branch creation did not trigger documentation publishing.
@silvanocerza silvanocerza merged commit 58e903a into master Oct 15, 2020
@silvanocerza silvanocerza deleted the scerza/fix-docs-publishing branch October 15, 2020 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Related to documentation for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants